Skip to content

ref(develop/spans): Expand ignoreSpans type and implementation specification #14571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 13, 2025

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Aug 6, 2025

DESCRIBE YOUR PR

Develop Docs PR that expands the specification for ignoreSpans:

  • add object accepting name and op
  • specify behaviour around dropping and reparenting of root vs child spans

closes #14568

IS YOUR CHANGE URGENT?

Help us prioritize incoming PRs by letting us know when the change needs to go live.

  • Urgent deadline (GA date, etc.):
  • Other deadline:
  • None: Not urgent, can wait up to 1 week+

SLA

  • Teamwork makes the dream work, so please add a reviewer to your PRs.
  • Please give the docs team up to 1 week to review your PR unless you've added an urgent due date to it.
    Thanks in advance for your help!

PRE-MERGE CHECKLIST

Make sure you've checked the following before merging your changes:

  • Checked Vercel preview for correctness, including links
  • PR was reviewed and approved by any necessary SMEs (subject matter experts)
  • PR was reviewed and approved by a member of the Sentry docs team

LEGAL BOILERPLATE

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

EXTRA RESOURCES

Copy link

vercel bot commented Aug 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Project Deployment Preview Comments Updated (UTC)
develop-docs Ready Preview Comment Aug 12, 2025 3:22pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
sentry-docs ⬜️ Ignored Preview Aug 12, 2025 3:22pm

@Lms24 Lms24 changed the title ref(spans): Expand ignoreSpans type and implementation specification ref(develop/spans): Expand ignoreSpans type and implementation specification Aug 6, 2025
@Lms24 Lms24 self-assigned this Aug 6, 2025
@Lms24 Lms24 requested review from cleptric and mydea August 6, 2025 15:10
Copy link
Member

@cleptric cleptric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't have an op once we move to span streaming.
I'm ok to not document this for now as it's JS only. We can keep the remaining changes though.

Copy link
Contributor

@coolguyzone coolguyzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Lms24 added a commit that referenced this pull request Aug 12, 2025
<!-- Use this checklist to make sure your PR is ready for merge. You may
delete any sections you don't need. -->

## DESCRIBE YOUR PR

Adds documentation for the new `ignoreSpans` option to be released with
SDK version 10.2.0.

SDK PR: getsentry/sentry-javascript#16820
Develop Docs spec change:
#14571

closes #14569 

## IS YOUR CHANGE URGENT?  

Help us prioritize incoming PRs by letting us know when the change needs
to go live.
- [ ] Urgent deadline (GA date, etc.): <!-- ENTER DATE HERE -->
- [ ] Other deadline: <!-- ENTER DATE HERE -->
- [ ] None: Not urgent, can wait up to 1 week+

## SLA

- Teamwork makes the dream work, so please add a reviewer to your PRs.
- Please give the docs team up to 1 week to review your PR unless you've
added an urgent due date to it.
Thanks in advance for your help!

## PRE-MERGE CHECKLIST

*Make sure you've checked the following before merging your changes:*

- [ ] Checked Vercel preview for correctness, including links
- [ ] PR was reviewed and approved by any necessary SMEs (subject matter
experts)
- [ ] PR was reviewed and approved by a member of the [Sentry docs
team](https://github.com/orgs/getsentry/teams/docs)

## LEGAL BOILERPLATE

<!-- Sentry employees and contractors can delete or ignore this section.
-->

Look, I get it. The entity doing business as "Sentry" was incorporated
in the State of Delaware in 2015 as Functional Software, Inc. and is
gonna need some rights from me in order to utilize my contributions in
this here PR. So here's the deal: I retain all rights, title and
interest in and to my contributions, and by keeping this boilerplate
intact I confirm that Sentry can use, modify, copy, and redistribute my
contributions, under Sentry's choice of terms.

## EXTRA RESOURCES

- [Sentry Docs contributor guide](https://docs.sentry.io/contributing/)

---------

Co-authored-by: Alex Krawiec <[email protected]>
@Lms24 Lms24 force-pushed the lms/ref-develop-ignorespans branch from 89721bf to 890aba7 Compare August 12, 2025 15:15
@Lms24 Lms24 requested review from cleptric and mydea August 12, 2025 15:17
} | {
name?: IgnoreSpanNamePattern;
attributes?: Record<string, EnhancedAttributeValueTypes>;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Empty Filter Configurations Allowed

The IgnoreSpanFilter union type allows empty objects {} because its second variant makes both name and attributes optional. This creates meaningless filter configurations that lack any criteria, potentially confusing SDK implementers and users. At least one of name or attributes should be required for a valid filter.

Fix in Cursor Fix in Web

@Lms24
Copy link
Member Author

Lms24 commented Aug 12, 2025

@cleptric @mydea I rewrote the filter object to have name and attributes as filter criteria. Omitted op completely, since I agree that we shouldn't specify anything that we'll end up removing "soon" anyway. Let me know if that works for you, otherwise I can split up the PR so that we get the parts in that can stay for sure.

For JS, we can transition over to attributes and continue supporting op for the time being but this is an implmentation detail.

@Lms24 Lms24 merged commit e73428c into master Aug 13, 2025
12 checks passed
@Lms24 Lms24 deleted the lms/ref-develop-ignorespans branch August 13, 2025 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update ignoreSpans specification
4 participants